Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

fix(handleRef): handle null ref correctly #487

Merged
merged 2 commits into from
Nov 19, 2018
Merged

Conversation

layershifter
Copy link
Member

A small fix for the handleRef ref utility.

typeof null // "object"

There is a similar condition in React's code: https://github.com/facebook/react/blob/e58ecda9a2381735f2c326ee99a1ffa6486321ab/packages/react-reconciler/src/ReactFiberCommitWork.js#L625

@layershifter
Copy link
Member Author

Should I add a record to CHANGELOG.md for this?

@codecov
Copy link

codecov bot commented Nov 19, 2018

Codecov Report

Merging #487 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #487   +/-   ##
=======================================
  Coverage   88.42%   88.42%           
=======================================
  Files          42       42           
  Lines        1451     1451           
  Branches      186      186           
=======================================
  Hits         1283     1283           
  Misses        163      163           
  Partials        5        5

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9bf84c1...15e53a2. Read the comment docs.

@layershifter layershifter added 🚀 ready for review 🧰 fix Introduces fix for broken behavior. labels Nov 19, 2018
Copy link
Member Author

@layershifter layershifter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The easiest case if when we will try use cloneElement and try to catch a component's ref:

const htmlInput = <input />

// ----

cloneElement(htmlInput, {
  ref: (node) => {
    // 1. Calling existing handler
    // A. We need to keep user's behaviour working, for example for 3rd party libs
    // B. `ref` is not a prop in React, so when `ref` is not passed it's `null` by default 
    //     and here it will fail without this fix
    handleRef(child.ref, node)
    // 2. Calling our handler
    this.handleInputRef(node)
  }
})

So, this fix is for a handler ref, not for a value.

@layershifter layershifter merged commit df2a657 into master Nov 19, 2018
@layershifter layershifter deleted the fix/handle-ref branch November 19, 2018 10:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🧰 fix Introduces fix for broken behavior. 🚀 ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants